[SuperEditor][SuperTextField] Improve RTL support (Resolves #2472) #2518
Conversation
| /// | ||
| /// Used to align and position the caret depending on whether | ||
| /// the text is RTL or LTR. | ||
| TextDirection? _contentTextDirection; |
There was a problem hiding this comment.
What if the text includes segments in both directions? Are we assuming that a text field only contains a single text direction?
There was a problem hiding this comment.
I guess the caret probably won't work correctly if have multiple directions in the same text field. I'll need to check if RenderParagraph returns the correct position.
There was a problem hiding this comment.
Is there a reason that these goldens use enough text to wrap lines? It seems to me that the multiple lines only gets in the way of understanding what these tests want to show - especially this one where we have black squares on both sides of the image, so it's not obvious what's being rendered. Just looks like a bunch of boxes.
Also, these file names should probably include some reference to the part we care about. So this file name makes it clear that we've got an ordered list item with RTL on iOS, but where do we expect the caret to sit? It's probably a good idea to mention that in the file name.
BTW, if this is an "ordered list item", why don't we have a numeral on it?
There was a problem hiding this comment.
Updated to use a single line and updated the test names.
BTW, if this is an "ordered list item", why don't we have a numeral on it?
I'm not sure why we have the numeral on Android and Linux, but not on the other platforms.
There was a problem hiding this comment.
Seems like a bug. Please file an issue if there's not something obvious to fix in this PR.
| void main() { | ||
| group('SuperEditor > RTL mode >', () { | ||
| testGoldensOnAllPlatforms( | ||
| 'inserts text and paints caret on the left side of paragraph', |
There was a problem hiding this comment.
Instead of just saying "left" and "right", we probably need to include a reference to both the side and the relative position. Maybe the words "upstream" and "downstream". Whatever makes the most sense. But since the point of these tests is to deal with cases where "left" and "right" mean different things in terms of content, we should make that clear.
|
@matthew-carroll It doesn't include breaking changes. |
[SuperEditor][SuperTextField] Improve RTL support. Resolves #2472
We have some issues in RTL support:
For
SuperEditor, we have code to infer the text direction from the text itself, but we weren't passing the text direction down theTextComponent, and we were also not passing it toLayoutAwareRichText. As the result, the caret position is computed as if we are in LTR.For
SuperTextField, we weren't inferring the text direction from the text.This PR makes the following changes:
LayoutAwareRichText.SuperTextField'stextAlignnullable, to choose the alignment based on the text direction, if the app doesn't enforce a text alignment.